Skip to content

test: Increase Email test coverage and enable component in random tests#10411

Open
gr8man wants to merge 6 commits into
codeigniter4:developfrom
gr8man:email-tests-coverage
Open

test: Increase Email test coverage and enable component in random tests#10411
gr8man wants to merge 6 commits into
codeigniter4:developfrom
gr8man:email-tests-coverage

Conversation

@gr8man

@gr8man gr8man commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Description
Ref: #9968

  • Added additional test cases for the Email class (covering header injection prevention, SMTP disconnection handling, UTF-8 filenames, and other edge cases).
  • Resolved global state pollution issues in namespace overrides by implementing cleanup in tearDown().
  • Enabled the Email component in the random tests execution configuration (random-tests-config.txt).
  • Fixed a PHPDoc type typo for the $newline property in Email.php.

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value (without duplication)
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@gr8man
gr8man force-pushed the email-tests-coverage branch from 71e52b3 to e22a54b Compare July 15, 2026 19:44

@michalsn michalsn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove the newly added tests that depend on the namespace-level native-function mocks, together with the global variables used to control them.

Although namespace-level function shadowing can be useful for isolated cases, using it here across nine native functions creates persistent process-level behavior and substantial global test state. This makes the tests difficult to isolate and maintain, and several mocks replace native behavior even when a test has not explicitly configured them.

Comment thread system/Email/Email.php Outdated
Comment on lines +185 to +200
@@ -197,7 +197,7 @@ class Email
*
* @see http://www.ietf.org/rfc/rfc822.txt
*
* @var "\n"|"\r\n"
* @var string

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're weakening annotations. Please revert.

{
use ReflectionHelper;

protected function tearDown(): void

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this method is needed, you have to call parent::tearDown().

@michalsn michalsn added the testing Pull requests that changes tests only label Jul 17, 2026
gr8man added a commit to gr8man/CodeIgniter4 that referenced this pull request Jul 17, 2026
@github-actions github-actions Bot removed the testing Pull requests that changes tests only label Jul 17, 2026
@gr8man
gr8man force-pushed the email-tests-coverage branch 3 times, most recently from 9af1c02 to a1d6bf0 Compare July 17, 2026 21:36
gr8man added a commit to gr8man/CodeIgniter4 that referenced this pull request Jul 17, 2026
@gr8man
gr8man force-pushed the email-tests-coverage branch from a1d6bf0 to a5a67e1 Compare July 17, 2026 21:43
@gr8man
gr8man force-pushed the email-tests-coverage branch from 6fffb0d to df7fcc7 Compare July 17, 2026 21:48
@gr8man

gr8man commented Jul 17, 2026

Copy link
Copy Markdown
Contributor Author

I cannot fix the PHPDoc type typo (@var "\r\n"|"n") directly in system/Email/Email.php because of an upstream issue with PHP-CS-Fixer's global_namespace_import rule.

Since Email.php has a namespace (CodeIgniter\Email), the fixer incorrectly treats the \n inside the PHPDoc string as a global class, stripping the backslash and erroneously injecting a use n; statement at the top of the file, which breaks the build.

@gr8man
gr8man requested a review from michalsn July 18, 2026 20:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants